Skip to content

Default to a single peer connection - #1113

Open
xianshijing-lk wants to merge 1 commit into
sxian/CLT-3302/bundle-the-publisher-offer-with-the-joinrequest-offer-with-joinfrom
sxian/CLT-3305/enable-single-peer-connection-by-default
Open

Default to a single peer connection#1113
xianshijing-lk wants to merge 1 commit into
sxian/CLT-3302/bundle-the-publisher-offer-with-the-joinrequest-offer-with-joinfrom
sxian/CLT-3305/enable-single-peer-connection-by-default

Conversation

@xianshijing-lk

Copy link
Copy Markdown
Contributor

Ticket 4 of the connect-latency series. Stacked on #1111 — retarget to main once #1110 and #1111 merge.

RoomOptions.singlePeerConnection has defaulted to false in Swift while client-sdk-js (defaults.ts:45) and rust-sdks (room/mod.rs:471) have both defaulted to true. Swift was the only one of the three opening two peer connections by default — two ICE and DTLS handshakes, and no access to any v1-only connect optimization.

Why this is the change that matters

Benchmarked on LiveKit Cloud staging, BM-CONN-001 / BM-CONN-003, 25 iterations + 5 warmups each:

Config wall p50 wall p90 D_WS p50 D_TRANSPORT p50
main + dual PC (ships today) 508 553 206 298
main + single PC 511 675 212 291
#1110+#1111 + dual PC 514 552 214 298
#1110+#1111 + single PC (this PR's default) 482 555 219 242

Read D_TRANSPORT_MS (join_recv → pc_connected); D_WS_MS is pure TLS/WS handshake and is flat across all four, so it is the noise floor.

~50 ms (−17%) off the transport phase. That is one round trip, matching the RTT implied by D_WS_MS ≈ 206 ms spanning ~3–4 RTTs. The saving is one RTT, so it scales with client-to-region latency — larger on the high-RTT paths that motivated this work.

The row that justifies the stack: single PC alone buys nothing (291 vs 298). The gain is entirely offer-with-join from #1111 — and #1111 only runs on the single-PC path, so without this flip it reaches nobody. Neither PR is worth much without the other.

Compatibility

/rtc/v1 requires LiveKit Cloud or LiveKit OSS ≥ 1.9.2. Older servers 404 the endpoint, and the existing serviceNotFound fallback (Room+Engine.swift) re-runs the connect on the legacy dual-PC path — verified by v1LocalhostFallback, which passes. The cost on an old server is one failed request, not a failed connect.

Opt out with RoomOptions(singlePeerConnection: false).

Testing

Against LiveKit Cloud staging:

PeerConnectionSignalingTests + RoomTests — 15/15 passed, both PC modes

Against local livekit-server 1.13.1: same suites green.

Builds verified on macOS, Mac Catalyst, iOS Simulator. swiftlint and swiftformat --lint clean. No API signature change — only default values, so api-check should be unaffected.

⚠️ Known pre-existing flake, worth deciding on before merge

dataChannel(mode: V1 Single PC) intermittently times out at PeerConnectionSignalingTests.swift:281. Measured rate against staging:

So this PR does not cause it — it is a pre-existing single-PC data-channel race. But it is the reason to think twice: today almost nobody runs single PC, and this change makes it the path every user takes, so a mode-specific flake becomes a mode-specific production issue.

My recommendation is to root-cause that timeout before or alongside merging this, rather than after. Happy to take it as a follow-up ticket — it looks like a data channel open race on the single-PC path, but I have not diagnosed it.

🤖 Generated with Claude Code

Matches client-sdk-js (`defaults.ts:45`) and rust-sdks (`room/mod.rs:471`),
which have both defaulted to single PC for some time. Swift was the only one
of the three still opening two peer connections by default, paying two ICE
and DTLS handshakes and unable to use any of the v1-only connect
optimizations.

Measured against LiveKit Cloud staging, 25 iterations per configuration:
the post-signal connect phase drops from 291ms to 242ms p50 (-17%) once the
publisher offer rides with the JOIN request. Single PC alone accounts for
none of that -- 291ms versus 298ms for dual PC -- so this change is only
worth anything stacked on offer-with-join, and offer-with-join reaches no
one without it.

Servers older than LiveKit OSS 1.9.2 do not serve /rtc/v1; the existing
serviceNotFound fallback re-runs the connect against the legacy path, so
the cost there is one failed request rather than a failed connect.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Devin Review

encryptionOptions = nil
reportRemoteTrackStatistics = false
singlePeerConnection = false
singlePeerConnection = true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Default connections intermittently lose data

Default singlePeerConnection enables a mode with reproducible data-channel timeouts. Data messages, RPC calls, and streams can fail for every default connection.

Prompt for agents
Keep the default on the stable dual-peer mode until the intermittent single-peer data-channel timeout is fixed. The failure reproduces in PeerConnectionSignalingTests.dataChannel when singlePeerConnection is true, and this PR makes that path the default for RoomOptions() and the parameterized initializer. Diagnose the channel-open or initial-offer race, add a repeatable regression test, then enable the default only after data messages reliably arrive.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant